Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improve parallelism of nightly wheel builds #1516

Merged
merged 6 commits into from
Jan 28, 2025
Merged

improve parallelism of nightly wheel builds #1516

merged 6 commits into from
Jan 28, 2025

Conversation

jameslamb
Copy link
Member

@jameslamb jameslamb commented Jan 23, 2025 •
edited
Loading

Description

Contributes to rapidsai/build-planning#136

Proposes the following:

  • having wheel-build-cuspatial depend on wheel-build-libcuspatial in nightly CI, not wheel-publish-libcuspatial
    • wheel-build-cuspatial just needs libcuspatial wheels from S3, not nightly PyPI
    • doing this reduces the end-to-end time for nightly builds (via increasing parallelism)

Notes for Reviewers

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@jameslamb jameslamb added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jan 23, 2025
@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Jan 23, 2025

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions github-actions bot added cmake Related to CMake code or build configuration Python Related to Python code libcuspatial Relates to the cuSpatial C++ library Java Related to Java code ci labels Jan 23, 2025
@jameslamb jameslamb mentioned this pull request Jan 23, 2025
Closed
@jameslamb
Copy link
Member Author

/ok to test

jameslamb
jameslamb commented Jan 23, 2025
View reviewed changes
python/cuproj/cuproj/cuprojshim/CMakeLists.txt Outdated
Comment on lines 123 to 124
PUBLIC "$<BUILD_INTERFACE:${CUPROJ_SOURCE_DIR}/include>"
PUBLIC "$<BUILD_INTERFACE:${CUPROJSHIM_SOURCE_DIR}/include>"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only manual changes in this PR... fixes this finding from cmake-lint:

python/cuproj/cuproj/cuprojshim/CMakeLists.txt:140,02: [E1122] Duplicate keyword argument PUBLIC

My read of https://cmake.org/cmake/help/latest/command/target_include_directories.html is that these forms of https://cmake.org/cmake/help/latest/command/target_include_directories.html are equivalent, so this is just a style thing and I don't think it should affect behavior at all.

jameslamb
jameslamb commented Jan 23, 2025
View reviewed changes
cpp/cmake/config.json Outdated Show resolved Hide resolved
jameslamb
jameslamb commented Jan 23, 2025
View reviewed changes
cpp/cmake/config.json Outdated Show resolved Hide resolved
@jameslamb jameslamb changed the title WIP: remove unnecessary libcuspatial wheel builds, enforce cmake-format and cmake-lint remove unnecessary libcuspatial wheel builds, enforce cmake-format and cmake-lint Jan 23, 2025
@jameslamb jameslamb marked this pull request as ready for review January 23, 2025 23:24
@jameslamb jameslamb requested review from a team as code owners January 23, 2025 23:24
vyasr
vyasr previously requested changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking for now on rapidsai/cugraph#4889 (review)

@bdice
Copy link
Contributor

bdice commented Jan 24, 2025

Yes, let's please split this up and do the CMake formatting/linting separately from the wheel build changes.

@jameslamb
Copy link
Member Author

will do

@jameslamb jameslamb mentioned this pull request Jan 24, 2025
Closed
@github-actions github-actions bot removed the Python Related to Python code label Jan 24, 2025
@github-actions github-actions bot removed the ci label Jan 24, 2025
@jameslamb jameslamb changed the title remove unnecessary libcuspatial wheel builds, enforce cmake-format and cmake-lint remove unnecessary libcuspatial wheel builds Jan 24, 2025
@github-actions github-actions bot removed libcuspatial Relates to the cuSpatial C++ library Java Related to Java code labels Jan 24, 2025
@jameslamb jameslamb requested a review from vyasr January 24, 2025 20:46
@jameslamb jameslamb changed the title remove unnecessary libcuspatial wheel builds improve parallelism of nightly wheel builds Jan 24, 2025
@jameslamb
Copy link
Member Author

Removed all the style-related stuff. This now just contains the one small change to improve parallelism in nightly builds.

@jameslamb
Copy link
Member Author

This is blocked until #1517 is merged.

@jameslamb jameslamb dismissed vyasr’s stale review January 27, 2025 21:59

all style-related stuff has been removed

@jameslamb
Copy link
Member Author

@vyasr I just dismissed your blocking review, now that all the CMake formatting stuff has been totally removed and this PR just has the 1-line shared-workflows change.

@jameslamb
Copy link
Member Author

Thanks very much for updating this after merging #1517 @mroeschke !

@jameslamb
Copy link
Member Author

/merge

@rapids-bot rapids-bot bot merged commit f1bd2b3 into rapidsai:branch-25.02 Jan 28, 2025
77 checks passed
@jameslamb jameslamb deleted the misc-packaging branch January 28, 2025 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdice bdice bdice approved these changes

@vyasr vyasr Awaiting requested review from vyasr

Assignees
No one assigned
Labels
cmake Related to CMake code or build configuration improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
cuSpatial
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jameslamb @bdice @vyasr @mroeschke